[PM-28555] Add idempotent sproc to create My Items collections #6801
Conversation
- DRY up collection arrangement - DRY up tests because bulk and non-bulk share same behavior - use EF native AddRange instead of bulk insert, because we expect smaller data sizes on self-host
|
Claude finished @eliykat's task in 2m 4s —— View job Code Review SummaryThis PR adds a stored procedure to create My Items collections idempotently, replacing manual collection creation logic with a more robust approach. The implementation is generally solid with good test coverage. Review DetailsNo critical issues found. The code is well-structured, properly tested, and follows established patterns in the codebase. The acknowledged TOCTOU risk is acceptable given the architectural decision to avoid database-level uniqueness constraints. Architecture NotesThe implementation uses three different strategies depending on the scenario:
This is intentional - the sproc provides idempotency for typical cases, while bulk operations optimize for throughput at the cost of slightly higher TOCTOU risk. Code Quality
Approved - Ready to merge after passing CI checks. |
|
New Issues (2)Checkmarx found the following issues in this Pull Request
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6801 +/- ##
==========================================
+ Coverage 55.72% 59.63% +3.90%
==========================================
Files 1949 1950 +1
Lines 86539 86536 -3
Branches 7725 7726 +1
==========================================
+ Hits 48227 51605 +3378
+ Misses 36506 33041 -3465
- Partials 1806 1890 +84 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-28555
📔 Objective
Add a sproc to:
After some iteration in #6791 , we are accepting some time-of-check time-of-use risk here. Architecture did not want to enforce uniqueness at the database level so we will keep it simple, filter out existing My Items collections in the sproc, and we can increase the isolation level later if it proves to be a problem.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes